fix(agents): preserve format guard recovery - #2985

Walkthrough

The change updates mirrored logging scripts and workflow templates. Optimizer workflows capture deduplication failures. Format guard workflows distinguish accepted dispatches from failed requests and manage the agents:format lease. Tests cover the failure path.

Changes

Workflow reliability updates

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant FormatGuard
  participant GitHubActions
  participant IssueLabels
  FormatGuard->>GitHubActions: Dispatch optimizer workflow
  GitHubActions-->>FormatGuard: Return CLI failure
  FormatGuard->>GitHubActions: Query matching runs after dispatch time
  GitHubActions-->>FormatGuard: Report accepted run or no matching run
  FormatGuard->>IssueLabels: Preserve or remove agents:format lease

Possibly related PRs

• stranske/Workflows#2956: Introduced the format-guard workflow modified by this change.
• stranske/Workflows#2979: Modified related format-guard and optimizer workflow behavior.
• stranske/Workflows#2981: Added related issue-format lease and dispatch handling.

Suggested labels: verify:compare

❌ Failed checks (1 warning)

openai

• Model: gpt-5.6-terra
• Verdict: CONCERNS
• Confidence: 84%
• Scores:
  • Correctness: 8.0/10
  • Completeness: 8.0/10
  • Quality: 8.0/10
  • Testing: 7.0/10
  • Risks: 7.0/10
• Summary: The PR implements all stated change areas: confirmed failed optimizer dispatches remove the format lease, accepted-but-CLI-failed dispatches preserve it, issue-dedup is made advisory despite bash -e, info logging correctly falls back to console.log, and the corresponding consumer templates are mirrored. The dedup shell handling is correct and the implementation is readable. However, the new run-list verification itself is not failure-tolerant under bash -e, so a lookup failure can bypass lease cleanup. The added workflow test coverage appears focused on source-level trigger behavior and does not demonstrate this polling/error-path recovery behavior.
• Concerns:
  • The new dispatch-acceptance polling path is executed under bash -e without guarding gh run list | jq. A transient GitHub CLI/API failure or jq failure during any poll exits the step immediately, before the failed agents:format lease is removed. This leaves the issue unable to retry, which is the recovery condition this change is intended to address.
  • Acceptance detection uses a timestamp with second-level precision plus a display-title substring match. An unrelated or prior matching run created within the same second could be treated as confirmation that the failed dispatch was accepted, preserving the lease unnecessarily. This is a relatively low-probability false-positive but affects recovery behavior.

anthropic

• Model: claude-sonnet-5
• Verdict: PASS
• Confidence: 72%
• Scores:
  • Correctness: 8.0/10
  • Completeness: 8.0/10
  • Quality: 7.0/10
  • Testing: 6.0/10
  • Risks: 7.0/10
• Summary: The PR addresses all four stated tasks: (1) release of the agents:format lease on genuine optimizer dispatch failure is implemented via a retry-based ambiguity check that avoids releasing the lease if the dispatch actually succeeded despite a non-zero CLI exit — this is a reasonable and correct approach; (2) issue-dedup advisory handling under bash -e is fixed by wrapping the python block with set +e/set -e, ensuring the dedup return code is captured instead of aborting the script; (3) info-level logging severity is preserved by routing non-error/non-warning levels to console.log instead of console.error; (4) consumer workflow templates were mirrored with matching changes and the template-drift-allowlist hashes were updated accordingly. Code quality is reasonable, though the new polling/ambiguity-detection logic in the format guard workflow is fairly intricate and could benefit from clearer inline documentation or dedicated tests. Test coverage additions are minimal (only +10 lines in one test file visible), so full acceptance-criteria command execution can't be independently confirmed from the diff, but per-guideline testing gaps alone don't block a PASS given the implementation appears functionally correct and CI signals (Python/JS tests, lint, format) for the merge commit are green. No major security or breaking-change risks identified.
• Concerns:
  • The retry/polling logic for detecting an 'ambiguous' dispatch (gh workflow run CLI failure but run actually accepted) adds complexity (5 retries x 2s sleep, jq-based JSON matching on createdAt/displayTitle) that is harder to verify deterministically in tests; correctness relies on GitHub API timing assumptions (string-based ISO8601 comparison, title matching '#NUMBER').
  • The diff for tests/workflows/test_agents_issue_optimizer_format_trigger.py only shows +10 lines with no visibility into what new scenario is covered (e.g., dispatch-failure-then-lease-release path is not obviously exercised based on the truncated diff).
  • No new/updated test asserting the info-level logging fix (console.log vs console.error) is visible in the diff for github-api-with-retry.test.js, though existing CI (JavaScript Tests) reportedly passed.
  • Manual verification of git diff --check and the two pytest/node test commands listed in acceptance criteria isn't directly confirmable from the diff alone, though CI summary indicates related test suites passed.